[PM-10929] Support Key Connector #959
Conversation
…g master key for key connector
|
No New Or Fixed Issues Found |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #959 +/- ##
=======================================
Coverage 58.43% 58.44%
=======================================
Files 195 196 +1
Lines 13406 13483 +77
=======================================
+ Hits 7834 7880 +46
- Misses 5572 5603 +31 ☔ View full report in Codecov by Sentry. |
| let master_key = MasterKey::generate(&mut rng); | ||
| let (user_key, encrypted_user_key) = master_key.make_user_key()?; | ||
| let keys = user_key.make_key_pair()?; | ||
|
|
||
| Ok(KeyConnectorResponse { | ||
| master_key: master_key.to_base64(), | ||
| encrypted_user_key: encrypted_user_key.to_string(), | ||
| keys, | ||
| }) |
There was a problem hiding this comment.
This is quite different compared to our current approach, but helps avoiding the weird steps in our ts project @jlf0dev.
There was a problem hiding this comment.
Perhaps I don't understand the key connector flow, but I had thought the encrypting keys were generated by key connect -- are they just stored there?
| @@ -79,6 +81,11 @@ impl<'a> ClientAuth<'a> { | |||
| make_register_tde_keys(self.client, email, org_public_key, remember_device) | |||
| } | |||
|
|
|||
| pub fn make_key_connector_keys(&self) -> Result<KeyConnectorResponse, CryptoError> { | |||
There was a problem hiding this comment.
Why not return a Result? Equivalently, why does this method return a specific error type rather than the base error enum?
There was a problem hiding this comment.
We're transitioning away from a single massive enum as that meshes badly with the new crate structure. And it's generally a better experience knowing what errors you get vs needing to validate all errors.
| let master_key = MasterKey::generate(&mut rng); | ||
| let (user_key, encrypted_user_key) = master_key.make_user_key()?; | ||
| let keys = user_key.make_key_pair()?; | ||
|
|
||
| Ok(KeyConnectorResponse { | ||
| master_key: master_key.to_base64(), | ||
| encrypted_user_key: encrypted_user_key.to_string(), | ||
| keys, | ||
| }) |
There was a problem hiding this comment.
Perhaps I don't understand the key connector flow, but I had thought the encrypting keys were generated by key connect -- are they just stored there?
| .0 | ||
| .auth() | ||
| .make_key_connector_keys() | ||
| .map_err(Error::Crypto)?) |
There was a problem hiding this comment.
If changed to a base error response, this map should be unnecessary, too
|
Key connector just stores your keys. The client still generates them. |
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-10929
📔 Objective
Add support for Key Connector.
KeyConnectortoinitialize_user_crypto.⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes